Skip to content

[muse_glimmer] Enable CP for the multimodal model - #4387

Open
jinsooihm wants to merge 3 commits into
pytorch:mainfrom
jinsooihm:muse-glimmer-cp
Open

[muse_glimmer] Enable CP for the multimodal model#4387
jinsooihm wants to merge 3 commits into
pytorch:mainfrom
jinsooihm:muse-glimmer-cp

Conversation

@jinsooihm

Copy link
Copy Markdown
Contributor

Summary

This PR enables CP for the multimodal muse_glimmer.

MuseGlimmerModel.preprocess_inputs builds vision_bank_indices_T: [T] from unsharded text tokens, which maps each token position to the index of vision encoder output that should be placed there.
Then, vision_bank_indices_T gets permuted and sharded by prepare_context_parallel_input jointly with tokens, so it can be used in the model forward to gather the vision encoder output (which is replicated on CP) after token is embedded.

Test Plan

TP2+SP vs TP2+CP2+SP: max loss difference 1.38e-5; max grad_norm difference 5.04e-4 from running loss_compare.py

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Aug 31, 2026
Build packed vision source indices before context-parallel token sharding and carry them through the same load-balancing permutation. Keep vision compute replicated across CP, then use a stateless sharding boundary to gather each local token shard while preserving TP and FSDP gradient semantics.

Generalize the shared vision attention layouts, keep multimodal embedding preparation on pipeline stage 0, and support the combined TP+CP+PP+SP topology without changing existing non-CP behavior or checkpoint keys.

Test Plan: 71 focused CPU tests; 2 distributed GPU tests; 10-step CP2xPP2 and TP2xCP2xPP2+SP real-process-group runs; exact 10-step deterministic loss and grad-norm parity; repository pre-commit hooks excluding the separately documented optional-dependency Pyrefly blocker.
@jinsooihm
jinsooihm marked this pull request as ready for review September 1, 2026 00:59
Comment thread torchtitan/models/common/multimodal.py Outdated
return vision_bank_indices_T.masked_fill(~vision_mask_T, -1)


class VisionScatter(Module):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question 1: are you putting it here because other multimodal models could share? Is there a vlm model in torchtitan that cannot use this to implement CP?

Question 2: do you create this stateless module only because you want _vision_scatter_config to perform spmd collectives at the module boundary? Fwiw we are moving away from such pattern because they couldn't express fused comm + computation. Could you put collectives INSIDE model code instead? I think we at least we could have a function that does vision scattering, instead of a module.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. In principle, this should allow CP on any VLM that has support for CP in the decoder.
  2. I will try to remove that.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tianyu-l We are moving away from doing spmd collective at the module boundary. But we currently can only swap modules. When you said, calling collectives inside model code, is this still a swappable one or a if/else condition inside the model code?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is only one redistribute required for this, where we SP shard vision_bank_indices_T. So this can be done in preprocess_inputs which already has parallel_dims and parallelism. So the scatter function does not need to do redistribute (note that SP sharding is also compatible with scattering using vision_bank_indices_T).

But I am not sure how this kind of parallel_dims dependent redistribute can be done without a module. Modules can remember it from parallelize like how it is done in ScaledBiasRowwiseLinear.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fegin

But we currently can only swap modules.

Didn't understand

When you said, calling collectives inside model code, is this still a swappable one or a if/else condition inside the model code?

Didn't understand either, lol. I think the latest commit LGTM.

@jinsooihm

But I am not sure how this kind of parallel_dims dependent redistribute can be done without a module. Modules can remember it from parallelize like how it is done in ScaledBiasRowwiseLinear.

That's good question. I think we should

  • either have spmd context provide the parallel info
  • or passing parallel info along the the model definition

I would prefer 1, if it can be done properly (thread local is good enough??) cc @pianpwk

@tianyu-l tianyu-l left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you plan to support other VLM soon?

)
if parallelism.spmd_backend == "spmd_types":
if (
parallelism.enable_sequence_parallel

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pianpwk we need this info from global context

@acisseJZhong

Copy link
Copy Markdown
Contributor

can qwen3.5 vl also benefit from the common utils here?

When looking into qwen3.5 CP, I found that we need to first do vision scatter -> CP shard. Is this what you are doing here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants